@llvm/pr-subscribers-lldb

Author: José Lira Junior (junior-jl)

diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index c7ce1b1258c196c..f601316a6f673ec 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -265,8 +265,6 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach {
         process_sp->SyncIOHandler(0, std::chrono::seconds(2));
 
         llvm::StringRef data = stream.GetString();
-        if (!data.empty())
-          result.AppendMessage(data);
         // If we didn't have a local executable, then we wouldn't have had an
         // executable module before launch.
         if (!exe_module_sp)
@@ -282,6 +280,8 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach {
               exe_module_sp->GetFileSpec().GetPath().c_str(), archname);
         }
         result.SetStatus(eReturnStatusSuccessFinishResult);
+        if (!data.empty())
+          result.AppendMessage(data);
         result.SetDidChangeProcessState(true);
       } else {
         result.AppendError(

I was about to tell you about the process launch -m shortcut, then I realised you added that yourself :) (I'm using it all the time now)

Upon implementing this change, two tests failed: lldb/test/Shell/Breakpoint/jit-loader_jitlink_elf.test and lldb/test/Shell/Breakpoint/jit-loader_rtdyld_elf.test.

This is actually good news then, we already have test coverage. Please re-order the expected output for those tests.

You can use a few specific terms to auto close an issue when a related PR lands: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests

Fixes #12345 for example.

Also could you include a bit more explanation of the problem in the commit message. The issue is great, but it'll save someone a few clicks if you just include a sentence or two more in the commit message.

I was about to tell you about the process launch -m shortcut, then I realised you added that yourself :) (I'm using it all the time now)

Haha, that's great to know! 🥹

This is actually good news then, we already have test coverage. Please re-order the expected output for those tests.

Done. ✅

Also could you include a bit more explanation of the problem in the commit message. The issue is great, but it'll save someone a few clicks if you just include a sentence or two more in the commit message.

I added more information. Now that you taught me that the PR description becomes the commit message in the end (in the other PR), I got a little worried of putting too much information, so please, let me know if I should add something else.

Opinions differ on what goes in a commit message but this seems fine to me, I'd just remove the sub-headings. My usual rule is that folks can choose not to read it if they don't want to, include as much information as you would want to see if you yourself had to come back to fix this change.

(which happens to me more than I'd like to admit :) )

Opinions differ on what goes in a commit message but this seems fine to me, I'd just remove the sub-headings. My usual rule is that folks can choose not to read it if they don't want to, include as much information as you would want to see if you yourself had to come back to fix this change.
(which happens to me more than I'd like to admit :) )

That's a good rule, thanks! Removed the sub-headings.

(if you don't have the permissions to do that, I can do it for you, let me know when you're ready)

I don't have write permissions here. Also, I tried using the --fixup flag on the second commit, does that mean the squashing is automatic or I need to squash the commits and push again?

Also, I tried using the --fixup flag on the second commit, does that mean the squashing is automatic or I need to squash the commits and push again?

Honestly I've never used that flag. I'm not sure it matters with the way llvm is setup. I guess if llvm-project used one of the commit's messages as the final message, it wouldn't choose a fixup commit, but we squash before merging anyway so it doesn't matter.